Skip to content

Merge no_panic_if(true), no_panic behavior#1526

Draft
ninehusky wants to merge 7 commits intoflux-rs:mainfrom
ninehusky:ninehusky-merge-no-panic-behavior
Draft

Merge no_panic_if(true), no_panic behavior#1526
ninehusky wants to merge 7 commits intoflux-rs:mainfrom
ninehusky:ninehusky-merge-no-panic-behavior

Conversation

@ninehusky
Copy link
Contributor

Closes #1521.

@ninehusky
Copy link
Contributor Author

ninehusky commented Mar 3, 2026

(ignore everything below, it's outdated now).

This is a pretty messy PR so far, but there are a lot of changes I had to make.

Right now, I'm in the middle of reorganizing the plumbing of genv::no_panic so that when it's doing the parent-walk, it returns when it finds either a parent item who's annotated with flux::no_panic, or an FnSig, in which case we can just return the fn_sig.no_panic(). While doing this generalization of no_panic to return an Expr instead of a boolean, I ran into a query cycle where conv_fn_sig is calling genv::no_panic, and vice versa.

So I think I'm violating some scheduling invariant. Maybe we need to move no_panic_if so that it's an attribute again, like with the regular no_panic? I don't remember all the details why the two annotations are handled so differently.

@ninehusky ninehusky force-pushed the ninehusky-merge-no-panic-behavior branch from 5a9ec09 to dde20f0 Compare March 6, 2026 01:37
@ninehusky
Copy link
Contributor Author

ninehusky commented Mar 6, 2026

Okay, I think getting a basic version that doesn't die is alright. Some todos to continue with:

  • support multi-closure inheritance (see the "grandchild" test of the last commit)
  • add the condition Nico told me about the two binders of the FnSig
  • write test to see if more complex no_panic_if expressions blow the system up, e.g., no_panic_if(T::foo_no_panic)
  • do a merge with the syntactic-inference PR as a stress test. ideally the alarm.rs closure false positive should shut up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Treat no_panic as no_panic_if(true)

1 participant